Skip to content

Post-merge-review: Fix template-no-nested-landmark: drop port-only section/region#2694

Merged
NullVoxPopuli merged 5 commits intoember-cli:masterfrom
johanrd:day_fix/template-no-nested-landmark
Apr 15, 2026
Merged

Post-merge-review: Fix template-no-nested-landmark: drop port-only section/region#2694
NullVoxPopuli merged 5 commits intoember-cli:masterfrom
johanrd:day_fix/template-no-nested-landmark

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 13, 2026

Summary

  • Removes <section> from LANDMARK_ELEMENTS and 'region' from LANDMARK_ROLES to match upstream
  • Port was over-flagging legitimate nested <section> elements

Test plan

  • <section><section>...</section></section> → valid
  • <div role="region"><div role="region">...</div></div> → valid
  • <main><nav>...</nav></main> → still flagged

Upstream excludes <section> from LANDMARK_ELEMENTS and 'region' from
LANDMARK_ROLES. Restore parity — port was over-flagging legitimate
nested sections.
@johanrd johanrd marked this pull request as ready for review April 13, 2026 16:23
@johanrd johanrd force-pushed the day_fix/template-no-nested-landmark branch from e18dc6a to 958884b Compare April 13, 2026 17:38
@johanrd johanrd changed the title Fix template-no-nested-landmark: drop port-only section/region Post-merge-review: Fix template-no-nested-landmark: drop port-only section/region Apr 13, 2026
'<template><header><div role="navigation"></div></header></template>',
'<template><div role="banner"><div role="navigation"></div></div></template>',

// `<section>` is not a landmark element per upstream, so nested sections are allowed.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upstream doesn't matter. what's MDN say?

Copy link
Copy Markdown
Contributor Author

@johanrd johanrd Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, changed the comment now — cites https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/HTML5.html directly: <section> only gets the region landmark role when it has an accessible name, otherwise it's generic.

// `<section>` is not a landmark element per upstream, so nested sections are allowed.
// `<section>` only gets the `region` landmark role when it has an accessible name
// (aria-label/aria-labelledby/title). Without one it has the generic role — see
// https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/section.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MDN is inconsistent on this — checked the HTML-AAM spec directly: https://www.w3.org/TR/html-aam-1.0/#el-section — "region role if the section element has an accessible name. Otherwise, the generic role."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tyty

@johanrd johanrd requested a review from NullVoxPopuli April 15, 2026 13:46
@NullVoxPopuli NullVoxPopuli merged commit 623705b into ember-cli:master Apr 15, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants